Replace library/busybox with lighter container repo - #20671
Merged
pondrejk merged 1 commit intoFeb 24, 2026
Conversation
lpramuk
added
CherryPick
lpramuk
force-pushed
the
replace_busybox_container_repo
branch
2 times, most recently
from
January 29, 2026 11:15
c0598a7 to
81b1e2a
Compare
lpramuk
force-pushed
the
replace_busybox_container_repo
branch
from
February 21, 2026 13:48
81b1e2a to
b5835c0
Compare
lpramuk
marked this pull request as ready for review
February 21, 2026 13:48
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSwitches the default container test image repository from library/busybox to jmalloc/echo-server and adjusts the CLI container management test to run the image in detached mode and correctly parse the container ID from command output for cleanup. Sequence diagram for updated container management test flowsequenceDiagram
actor Tester
participant TestCase
participant Config
participant CLI
participant ContainerRuntime
participant Registry
Tester->>TestCase: run test_container_management
TestCase->>Config: get container.upstream_name
Config-->>TestCase: jmalloc/echo-server (default)
TestCase->>CLI: container run --detached jmalloc/echo-server
CLI->>ContainerRuntime: create and start container (detached)
ContainerRuntime->>Registry: pull jmalloc/echo-server
Registry-->>ContainerRuntime: image layers
ContainerRuntime-->>CLI: stdout with container_id
CLI-->>TestCase: raw stdout
TestCase->>TestCase: parse container_id from stdout
TestCase->>CLI: container stop container_id
CLI->>ContainerRuntime: stop container
ContainerRuntime-->>CLI: stopped
CLI-->>TestCase: success
TestCase->>CLI: container rm container_id
CLI->>ContainerRuntime: remove container
ContainerRuntime-->>CLI: removed
CLI-->>TestCase: success
TestCase-->>Tester: test passed
Class diagram for container configuration validation changesclassDiagram
class Validator {
+string key
+bool must_exist
+type is_type_of
+any default
}
class ContainerConfig {
+string upstream_name
+list~string~ alternative_upstream_names
}
class ConfigRegistry {
+ContainerConfig container
+get_value(key) any
+set_value(key, value) void
}
ConfigRegistry o-- ContainerConfig : has
ConfigRegistry ..> Validator : uses
class UpstreamNameValidator {
+string key = container.upstream_name
+bool must_exist = true
+type is_type_of = str
+string default = jmalloc/echo-server
}
UpstreamNameValidator --|> Validator
Flow diagram for resolving default container image repositoryflowchart TD
A[Start test
read container config] --> B{container.upstream_name
configured?}
B -- Yes --> C[Use configured
container.upstream_name]
B -- No --> D[Apply default
jmalloc/echo-server]
C --> E[Run container tests
using selected repo]
D --> E[Run container tests
using selected repo]
E --> F[Cleanup containers]
F --> G[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The change from
result.stdout[0]toresult.stdout.split()[0]assumesstdoutis now a string rather than a list; please confirm and handle both cases (or enforce one type) to avoid type errors across differentexecuteimplementations. - Since
docker run -dalready prints the container ID, you can simplify the cleanup logic by capturing that ID directly from thedocker runresult instead of parsingdocker ps -a | grep ..., which is more brittle and may misbehave if multiple containers match.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change from `result.stdout[0]` to `result.stdout.split()[0]` assumes `stdout` is now a string rather than a list; please confirm and handle both cases (or enforce one type) to avoid type errors across different `execute` implementations.
- Since `docker run -d` already prints the container ID, you can simplify the cleanup logic by capturing that ID directly from the `docker run` result instead of parsing `docker ps -a | grep ...`, which is more brittle and may misbehave if multiple containers match.
## Individual Comments
### Comment 1
<location> `robottelo/config/validators.py:122` </location>
<code_context>
must_exist=True,
is_type_of=str,
- default='library/busybox',
+ default='jmalloc/echo-server',
),
Validator(
</code_context>
<issue_to_address>
**🚨 issue (security):** Consider pinning the container image to a specific tag instead of relying on an unversioned default.
This change shifts the default from an official image to a third‑party one and leaves the tag effectively unpinned (likely `latest`), which can introduce non-deterministic behavior and security/behavior drift as the upstream image changes. If this default is intended to be stable and reproducible, please pin a specific tag (e.g. `jmalloc/echo-server:<version>`) or clearly document that an unpinned, flexible image is expected here.
</issue_to_address>
### Comment 2
<location> `tests/foreman/cli/test_container_management.py:106` </location>
<code_context>
f'docker ps -a | grep {repo["published-at"]}'
)
- container_id = result.stdout[0].split()[0]
+ container_id = result.stdout.split()[0]
module_container_contenthost.execute(f'docker stop {container_id}')
module_container_contenthost.execute(f'docker rm {container_id}')
</code_context>
<issue_to_address>
**issue:** Guard against empty `stdout` before splitting to avoid errors in cleanup
The previous version indexed `stdout[0]` assuming a list; now we call `stdout.split()[0]` assuming a non-empty string. If `docker ps -a | grep {repo["published-at"]}` matches nothing, `stdout` can be empty and `stdout.split()[0]` will raise `IndexError`, obscuring the real issue. Please either assert `result.status == 0` and that `result.stdout.strip()` is non-empty before splitting, or explicitly handle the "no container found" case and fail the test with a clear message.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
chris1984
reviewed
Feb 23, 2026
chris1984
left a comment
chris1984
left a comment
Member
There was a problem hiding this comment.
What are the testing times with jmalloc/echo-server compared to library/busybox? Is there anything we are losing by switching other than getting shorter sync times?
Contributor
We are loosing some "Schema Version 1" manifests, which @lpramuk, are you aware of any similar-sized repo with V1 + V2 manifests? |
lpramuk
force-pushed
the
replace_busybox_container_repo
branch
from
February 23, 2026 14:10
b5835c0 to
c56deec
Compare
Contributor
Author
@chris1984 I tested a test collection of all affected tests, (totaling to 107 tests): You can see that total execution time dropped to 50% by 1hr 15min. |
Contributor
Author
|
@vsedmik What about keeping |
Contributor
I'm good with that. 👍 |
vsedmik
approved these changes
Feb 23, 2026
Contributor
Author
|
Collaborator
|
PRT Result |
Satellite-QE
added
the
PRT-Passed
pondrejk
approved these changes
Feb 24, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Feb 24, 2026
(cherry picked from commit 8ac73bc)
This was referenced Feb 24, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Feb 24, 2026
(cherry picked from commit 8ac73bc)
lpramuk
added a commit
to lpramuk/robottelo
that referenced
this pull request
Feb 24, 2026
(cherry picked from commit 8ac73bc)
lpramuk
added a commit
to lpramuk/robottelo
that referenced
this pull request
Feb 24, 2026
(cherry picked from commit 8ac73bc)
lpramuk
added a commit
that referenced
this pull request
Feb 25, 2026
lpramuk
added a commit
that referenced
this pull request
Feb 25, 2026
lpramuk
added a commit
that referenced
this pull request
Feb 25, 2026
rmynar
pushed a commit
to rmynar/robottelo
that referenced
this pull request
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Statement
Gradually over time the
library/busyboxrepo is becoming more and more huge.The problem with repo sync arises especially in IPv6 environment where the traffic goes over IPv6-to-4 proxy.
Testing revealed that syncing over IPv4 takes approx. 10 min while syncing over IPv6 takes 25 min and tends to time out quite often.
Solution
Find more suitable replacement for testing container repo target
Related Issues
SAT-41831
Summary by Sourcery
Update container test configuration to use a lighter upstream image and adjust test execution accordingly.
Enhancements:
Tests: